-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Conversation
Hey @leezu , Thanks for submitting the PR
CI supported jobs: [centos-cpu, windows-gpu, windows-cpu, sanity, clang, unix-cpu, website, centos-gpu, edge, miscellaneous, unix-gpu] Note: |
Hi @leezu, are you trying to include a unreleased version of oneDNN to MXNet v1.8? Or you just want to verify the fix? We're asking oneDNN team for a v1.6.4 patch release. |
Could you provide more details on what fixes are missing from oneapi-src/oneDNN@5ce95ef that will go into 1.6.4? I think it's up to @samskalicky as release manager to decide if he just includes oneapi-src/oneDNN@5ce95ef or wait for 1.6.4 (or don't include the fix at all) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have a test now that is passing with this fix?
We don't have a test in 1.x branch. One can construct a test by first calling |
Ok, np. did we verify the fix actually fixes the issue in 1.8.x manually? |
Let's see if #19260 results can show that the fix really fixes the issue. Update: It seems, that the failing tests don't exist on 1.8, so the PR is irrelevant. |
Thanks @leezu do you plan to open another PR for this fix on v1.x as well? |
@mxnet-bot run ci [unix-gpu] |
Jenkins CI successfully triggered : [unix-gpu] |
@bartekkuncer @pioy looks like the CI is passing now. Is this PR ready to merge? Can you review/approve it? |
@samskalicky Change looks good now. Ready to be merged. |
This change is included in v1.x as part of #19276 |
Include fix for #19185 (comment)
Thank you @pioy
Fix verified via CI run in #19185: https://jenkins.mxnet-ci.amazon-ml.com/blue/organizations/jenkins/mxnet-validation%2Fcentos-cpu/detail/PR-19185/2/pipeline/